Fix #7009: Remove leftover .msi and msi.log after update #8980
Conversation
Ensure leftover installer and msi.log are removed after external manual or internal updates. Improve logging on delete/copy failures. Fixes nextcloud#7009. Signed-off-by: chrip <[email protected]>
chrip
requested review from
Aiiaiiio,
camilasan,
i2h3,
mgallien and
nilsding
as code owners
|
Artifact containing the AppImage: nextcloud-appimage-pr-8980.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
Aiiaiiio
left a comment
Aiiaiiio
left a comment
There was a problem hiding this comment.
Nice one! Please see my comments as suggestions.
src/gui/updater/ocupdater.cpp
Outdated
| } | ||
| } | ||
| // Also try to remove the msi log file (created when running msiexec) | ||
| QString msiLogFileName = cfg.configPath() + "msi.log"; |
There was a problem hiding this comment.
I would consider using a constant for msi.log.
There was a problem hiding this comment.
Thanks, I’ve added a separate commit introducing a constant msiLogFileNameC as suggested.
| if (QFile::remove(msiLogFileName)) { | ||
| qCInfo(lcUpdater) << "Removed msi log file:" << msiLogFileName; | ||
| } else { | ||
| qCWarning(lcUpdater) << "Failed to remove msi log file:" << msiLogFileName; |
There was a problem hiding this comment.
Would it be good to check whether the current platform is Windows? I wouldn't expect an msi log on any other system.
There was a problem hiding this comment.
Hi @Aiiaiiio, thanks for the suggestion! From what I see, the changes are in NSISUpdater::wipeUpdateData() which is a Windows-specific subclass of OCUpdater. So this code isn’t compiled on other platforms than Windows and no additional platform check should be needed.
Signed-off-by: chrip <[email protected]>
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
3 participants
Fixes #7009
Ensure leftover Windows installers (.msi) and the msiexec log (msi.log) are removed after updates,
both for internal updater flow and for externally performed (manual) installs.
What I changed
How to test
Notes
Fixes #7009